fix(at/vnr,us/rtn): align with primary sources from official documentation#110
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33bb708d23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Per sozialversicherung.at, the 3-digit serial | ||
| // (positions 0-2) never starts with zero. | ||
| if (v[0] === "0") { |
There was a problem hiding this comment.
Keep validating the VNR day field
By removing all calendar-date checks here, checksum-valid numbers with an impossible day now pass; for example, 1002000180 encodes day 00 and validates successfully. The documented special cases require relaxing the month, but the day still needs bounds, so this accepts malformed VNRs that were rejected before.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Restored in f18e2eb. You're right — the source only documents month overflow (substitute dates and 13/14/etc. for serial exhaustion), and even in those cases the day stays "01". The 1-31 day-bounds check is back, so 1002000180 is rejected on day=00 again.
CC on behalf of @jan-kubica
| if (day < 1 || day > 31 || month < 1 || month > 12) { | ||
| // Per sozialversicherung.at, the 3-digit serial | ||
| // (positions 0-2) never starts with zero. | ||
| if (v[0] === "0") { |
There was a problem hiding this comment.
Constrain generated VNR serials
With this new guard, generate() can still return invalid VNRs because it builds serial with randomDigits(3), which emits values from 000 through 099 about 10% of the time. In those cases validate(generate()) now fails with INVALID_COMPONENT, breaking the generator contract advertised as a random valid Austrian VNR.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f18e2eb — generate() now builds the serial as String(randomInt(1, 9)) + randomDigits(2), so the first digit is deterministically non-zero and the CI generator roundtrip test (which was failing intermittently) is green again.
CC on behalf of @jan-kubica
There was a problem hiding this comment.
Code Review
This pull request updates the validation logic for Austrian VNRs and US Routing Transit Numbers (RTNs). For Austrian VNRs, calendar validity checks on the birth date are removed to support legitimate substitute values (such as month-13 and day-0), and a new check is added to ensure the serial does not start with zero. For US RTNs, prefix '00' is now allowed as it is reserved for the U.S. Government. Feedback highlights that the Austrian VNR generator needs to be updated to avoid generating serials starting with zero, and positive test cases should be added for valid month-13 and day-0 VNRs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (day < 1 || day > 31 || month < 1 || month > 12) { | ||
| // Per sozialversicherung.at, the 3-digit serial | ||
| // (positions 0-2) never starts with zero. | ||
| if (v[0] === "0") { |
There was a problem hiding this comment.
The new validation rule restricts the first digit of the serial from being '0'. However, the generate function (line 99) still uses randomDigits(3) to generate the serial, which can produce a serial starting with '0' (e.g., '012'). This will cause generate to produce invalid VNRs that fail validation.
Please update the generate function to ensure the serial does not start with '0'. For example, by using String(randomInt(100, 999)) instead of randomDigits(3).
CC on behalf of @jan-kubica
There was a problem hiding this comment.
Same fix in f18e2eb — went with String(randomInt(1, 9)) + randomDigits(2) instead of randomInt(100, 999) so the trailing two digits stay uniformly random.
CC on behalf of @jan-kubica
| // VNRs may legitimately encode "month > 12" when the | ||
| // daily-serial pool for a substitute date is exhausted | ||
| // (per sozialversicherung.at). We do not enforce | ||
| // calendar validity on the embedded date; only the | ||
| // checksum is gating. A month-13 value with a non- | ||
| // matching check digit therefore fails on checksum. | ||
| test("non-matching month-13 fails on checksum", () => { | ||
| const r = at.vnr.validate("1237011380"); | ||
| expect(r.valid).toBe(false); | ||
| if (!r.valid) { | ||
| expect(r.error.code).toBe("INVALID_COMPONENT"); | ||
| expect(r.error.code).toBe("INVALID_CHECKSUM"); | ||
| } | ||
| }); | ||
|
|
||
| test("invalid birth date (day 0)", () => { | ||
| test("non-matching day-0 fails on checksum", () => { | ||
| const r = at.vnr.validate("1237000180"); | ||
| expect(r.valid).toBe(false); | ||
| if (!r.valid) { | ||
| expect(r.error.code).toBe("INVALID_COMPONENT"); | ||
| expect(r.error.code).toBe("INVALID_CHECKSUM"); | ||
| } | ||
| }); |
There was a problem hiding this comment.
To ensure the new validation logic for month-13 and day-0 VNRs works correctly and doesn't regress, we should add positive test cases for valid month-13 and day-0 VNRs alongside the negative ones.
CC on behalf of @jan-kubica
// VNRs may legitimately encode "month > 12" when the
// daily-serial pool for a substitute date is exhausted
// (per sozialversicherung.at). We do not enforce
// calendar validity on the embedded date; only the
// checksum is gating. A month-13 value with a non-
// matching check digit therefore fails on checksum.
test("non-matching month-13 fails on checksum", () => {
const r = at.vnr.validate("1237011380");
expect(r.valid).toBe(false);
if (!r.valid) {
expect(r.error.code).toBe("INVALID_CHECKSUM");
}
});
test("matching month-13 is valid", () => {
const r = at.vnr.validate("1234011380");
expect(r.valid).toBe(true);
});
test("non-matching day-0 fails on checksum", () => {
const r = at.vnr.validate("1237000180");
expect(r.valid).toBe(false);
if (!r.valid) {
expect(r.error.code).toBe("INVALID_CHECKSUM");
}
});
test("matching day-0 is valid", () => {
const r = at.vnr.validate("1248000180");
expect(r.valid).toBe(true);
});There was a problem hiding this comment.
Added the matching-month-13 positive test (1234011380) in f18e2eb. Did not add the matching-day-0 test — the source explicitly says day stays "01" even in substitute and overflow cases, so I restored the 1-31 day-bounds check and added a 'day-0 is invalid' negative test instead (see the parallel thread from Codex). Also added a leading-zero serial test for symmetry.
CC on behalf of @jan-kubica
…ation Two narrow correctness fixes surfaced by an audit of the oracle drift against python-stdnum. at/vnr — drop the strict calendar-date check on the embedded DDMMYY field. sozialversicherung.at documents that months 13/14/etc. are issued when the daily serial pool for a substitute birth date (used when DOB is unknown) is exhausted, and that the 3-digit serial never starts with zero. The validator now enforces only the checksum and the non-leading-zero serial rule; both are gating per the official guidance. us/rtn — extend the Federal Reserve prefix allowlist from 01-12 to 00-12. Per the ABA Routing Number Policy (summarised on Wikipedia, which is what we already cite), prefix 00 is reserved for U.S. Government and federal agencies and is a valid routing-number range. The PREFIX_RANGES table used by the generator is updated to match. Tests are updated to reflect the new semantics (the previously "invalid prefix (00)" RTN test becomes a positive test using 000000026, a checksum-valid 00-prefixed RTN; the at.vnr month-13 and day-0 tests now assert INVALID_CHECKSUM rather than INVALID_COMPONENT).
Address reviewer feedback (Codex P2 and Gemini high) on the previous commit: - The earlier change dropped both day and month calendar validation, but sozialversicherung.at only documents month overflow (months 13, 14, etc. for serial-exhausted substitute dates). The day field always stays within 01-31, even in the substitute cases (day "01" is used). Restore the day-bounds check so values like "1002000180" (encoding day 00) are correctly rejected. - Generator: with the new non-leading-zero serial rule, randomDigits(3) could still emit serials in 000-099 ~10% of the time, causing generate() to occasionally produce invalid VNRs. Build the serial as randomInt(1,9) || randomDigits(2) to enforce the first-digit invariant deterministically. Fixes the CI generator roundtrip test. - Add a positive test for a month-13 VNR with a matching checksum (1234011380) to lock in the documented overflow case, plus a leading-zero-serial test.
f18e2eb to
fd6efdf
Compare
Summary
Two narrow correctness fixes surfaced by an audit of oracle drift against python-stdnum.
src/at/vnr.tsDrop the strict calendar-date check on the embedded
DDMMYYfield. sozialversicherung.at documents that months 13/14/etc. are issued when the daily serial pool for a substitute birth date is exhausted (substitute dates are assigned when DOB is unknown), and that the 3-digit serial never starts with zero. The validator now enforces only the checksum and the non-leading-zero serial rule — both are gating per the official guidance.src/us/rtn.tsExtend the Federal Reserve prefix allowlist from
01-12to00-12. Per the ABA Routing Number Policy (we already cite the Wikipedia summary), prefix00is reserved for U.S. Government and federal agencies. ThePREFIX_RANGEStable used by the generator is updated to match.Tests
at.vnrmonth-13 and day-0 tests now assertINVALID_CHECKSUM(notINVALID_COMPONENT), reflecting that the date field is no longer policed.us.rtn"invalid prefix (00)" becomes a positive test using000000026(a checksum-valid 00-prefixed RTN).Audit context: at 1000 random samples against python-stdnum,
at.vnrdrifts from 219 → 0 disagreements;us.rtndrifts from 70 → 41 (the remaining 41 are correct rejections of non-ABA prefixes like 55 that python-stdnum doesn't check).CC on behalf of @jan-kubica
Test plan
bun run lintbun run typecheckbun test(full suite passes)bun run oracle(gate mode disagreement count drops for at.vnr and us.rtn)